-
Notifications
You must be signed in to change notification settings - Fork 25
feat(networking): Implement support for UPnP #444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: unstable
Are you sure you want to change the base?
Conversation
|
Still in draft because not sure how to respond to events. |
|
Nice start! Regarding the events, I think we have to do #255 first (now that I think about it again). After all, it makes no sense to get external ports via UPnP if we can not advertise them. Let me know if you want to take a look at #255 as well (no pressure though!). Else, I can tackle it in the next few days and let you know when it's ready. |
Hi, sorry for not responding sooner, had something urgent to take care off. Sure, I can take a look, I will write questions / ideas in that issue then. |
|
Lighthouse also has this part: Where they "manually" construct port mappings for addresses they know are listening to (provided from the config). Not sure if we also need it or it will be caught by the "regular" libp2p behaviour |
|
@dknopik @petarjuki7 how about we continue this? |
|
@diegomrsantos sure, feel free to continue on this already, I'll focus on testing the release for now |
I meant @petarjuki7 :) |
5cb9b63 to
588f715
Compare
|
@petarjuki7 As this is not a fix and will not end up in the first mainnet release, |
588f715 to
93c84d7
Compare
93c84d7 to
588f715
Compare
|
@petarjuki7 is this ready for review? |
Sorry, hadn't seen this sooner, yes it is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements UPnP support to automatically establish external port mappings for the Anchor network client. Users can opt-out of UPnP with the --disable-upnp CLI flag.
- Added UPnP behavior to the network stack with event handling for port mapping
- Introduced CLI option to disable UPnP functionality
- Configured UPnP to update ENR ports when external addresses are mapped
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| anchor/network/src/network.rs | Implements UPnP event handling and ENR port updates |
| anchor/network/src/config.rs | Adds upnp_enabled configuration field |
| anchor/network/src/behaviour.rs | Integrates UPnP behavior into the network behavior stack |
| anchor/network/Cargo.toml | Adds upnp feature to libp2p dependency |
| anchor/client/src/config.rs | Maps CLI disable_upnp flag to network configuration |
| anchor/client/src/cli.rs | Adds --disable-upnp CLI argument |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Issue Addressed
Addresses issue #407
Proposed Changes
Added
libp2p::upnpfunctionality.Changed the CLI arguments so the user can opt-out of upnp with
--disable-upnp.Listening to
upnpevents.Additional Info
This PR currently doesn't do anything on upnp events regarding updating ENR ports.
Relates to #255.